Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

サインアップ機能 #1

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

サインアップ機能 #1

wants to merge 10 commits into from

Conversation

madokaaaaa03
Copy link
Collaborator

@madokaaaaa03 madokaaaaa03 commented Sep 27, 2024

受講生の確認事項

  • 画面をブラウザで実際に開いてテスト要件の画面と機能の動作確認をした(動作が分からない場合講師からスクリーンショットの提出を求めることがあります)
  • 作成したモデルを全てDjango管理画面に登録した
  • テスト要件のテストを全て実装した
  • CI が全て通った

1次レビュアーの確認事項

Copy link

black(フォーマッタ)のチェックに失敗しました。CI実行のログを確認して修正し,再度コミット・プッシュしてください。

Copy link

black(フォーマッタ)のチェックに失敗しました。CI実行のログを確認して修正し,再度コミット・プッシュしてください。

Copy link

github-actions bot commented Nov 1, 2024

Django Unit Testが失敗しました。実行ログを確認して修正し,再度コミット・プッシュしてください。

Copy link

github-actions bot commented Nov 1, 2024

black(フォーマッタ)のチェックに失敗しました。CI実行のログを確認して修正し,再度コミット・プッシュしてください。

@madokaaaaa03 madokaaaaa03 marked this pull request as ready for review December 16, 2024 15:11

</body>
</html>
<<!DOCTYPE html>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

<が1つ多くついているので修正お願いします。


{% block content %}
<form method="post">
{{ form.as_p }} ← formにはSignupFormのインスタンスが入っている。as_pとすることで、各input要素がpタグで囲まれた状態で表示される。
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

←formなどの部分はアプリとは関係ないと思うのでコメントアウトした方がいいと思います

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.DS_Storeが差分ファイルに入ってしまっています
.gitignoreの使い方を確認し、.DS_Storeが追跡する差分に入らないように、追記をするといいと思います!

参考📚

@@ -1 +1,6 @@
# from django.contrib import admin

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

要らなそうなコメントアウト?
細かいですが、不必要なコメントアウトは削除しちゃってください!

# from django.test import TestCase


# class TestSignupView(TestCase):
# def test_success_get(self):
# def test_success_get(self):

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

なるほど、一つ一つチェックしながらテスト書いていたんですね!
漏れなく実装できていていいと思います!

このタイミングではなくてもいいかもしれないですが、コードは全て意味があり意味のないコードは一通り開発が終わってPR出す時には含めないようにするのがいいと思うので、
必要なくなったら削除しちゃってくださいね!

# from .models import Like, Tweet


class TestHomeView(TestCase): # 不安
Copy link

@lokochaol lokochaol Jan 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

不安のコメントが笑
不安ですよね、でもよく書けていると思います!
理解が不安だったり、テキストで解決しないなと思う時は、自分含め修了生どなたでも画面共有等しながらアドバイスもらえるかと思うので、
アポ取ってみてくださいね!
自分もオフィスに行って見てもらったりしていたので!

ここも一応!不要なコメントは削除お願いします!


{% block content %}
<form method="post">
{{ form.as_p }} # formにはSignupFormのインスタンスが入っている。as_pとすることで、各input要素がpタグで囲まれた状態で表示される。

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

これとかもrunserverしたときに画面見てもらえればわかるのですが、
サイトに表示されてしまっているので、削除しちゃった方がいいかと思います!

@lokochaol
Copy link

PR作成お疲れ様でした、いい感じです!
あと細かいところだけ、
不要なコメントアウトの削除お願いします!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants